"Derrick Stolee via GitGitGadget" <[email protected]> writes:

> From: Derrick Stolee <[email protected]>
>
> In a previous change, the --negotiation-restrict command-line option of
> 'git fetch' was added as a synonym of --negotiation-tips. Both of these
> options restrict the set of 'haves' the client can send as part of
> negotiation.
>
> This was previously not available via a configuration option. Add a new
> 'remote.<name>.negotiationRestrict' multi-valued config option that
> updates 'git fetch <name>' to use these restrictions by default.
>
> If the user provides even one --negotiation-restrict argument, then the
> config is ignored.
>
> Signed-off-by: Derrick Stolee <[email protected]>
> ---
>  Documentation/config/remote.adoc | 16 ++++++++++++++++
>  builtin/fetch.c                  | 24 ++++++++++++++++++++++--
>  remote.c                         |  6 ++++++
>  remote.h                         |  1 +
>  t/t5510-fetch.sh                 | 22 ++++++++++++++++++++++
>  5 files changed, 67 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
> index 91e46f66f5..5e8ac6cfdd 100644
> --- a/Documentation/config/remote.adoc
> +++ b/Documentation/config/remote.adoc
> @@ -107,6 +107,22 @@ priority configuration file (e.g. `.git/config` in a repository) to clear
>  the values inherited from a lower priority configuration files (e.g.
>  `$HOME/.gitconfig`).
>  
> +remote.<name>.negotiationRestrict::
> +	When negotiating with this remote during `git fetch` and `git push`,
> +	restrict the commits advertised as "have" lines to only those
> +	reachable from refs matching the given patterns.  This multi-valued
> +	config option behaves like `--negotiation-restrict` on the command
> +	line.
> ++
> +Each value is either an exact ref name (e.g. `refs/heads/release`) or a
> +glob pattern (e.g. `refs/heads/release/*`).  The pattern syntax is the
> +same as for `--negotiation-restrict`.
> ++
> +These config values are used as defaults for the `--negotiation-restrict`
> +command-line option.  If `--negotiation-restrict` (or its synonym
> +`--negotiation-tip`) is specified on the command line, then the config
> +values are not used.

This is a tangent, but I wonder what happens when this is set in
/etc/gitconfig or ~/.gitconfig by mistake.  I personally do not
think of any good reason to set it in either of these two places,
so it might be fine to declare that we read this only from local
configuration file or "git -c var=val" command line, but alternative
that is easier to implement would be to allow for a variable
definition syntax that allows you to say "forget everything you read
so far, clear this multi-valued variable", e.g.

    == in /etc/gitconfig ==
    [remote "origin"]
	negotiationRestrict = refs/pull/*

    == in .git/config ==
    [remote "origin"]
	# clear them
	negotiationRestrict =
	negotiationRestrict = refs/heads/*
	negotiationRestrict = refs/tags/*

or something like that, perhaps?

It is a shame that our configuration framework do not allow
specifying their meanings and semantics to variables like
parse-options do (where OPT_STRING_LIST naturally allows
--no-negotiation-restrict to act as a way to clear the deck).
Because there is no official way to programatically declare that
remote.<name>.negotiationRestrict is a multi-valued variable whose
values are stored in a string-list, the config callback needs to 
be coded to implement the behaviour for each variable X-<.

"Derrick Stolee via GitGitGadget" <[email protected]> writes:

> +`--negotiation-require=<revision>`::
> +	Ensure that the given ref tip is always sent as a "have" line
> +	during fetch negotiation, regardless of what the negotiation
> +	algorithm selects.  This is useful to guarantee that common
> +	history reachable from specific refs is always considered, even
> +	when `--negotiation-restrict` restricts the set of tips or when
> +	the negotiation algorithm would otherwise skip them.
> ++
> +This option may be specified more than once; if so, each ref is sent
> +unconditionally.
> ++
> +The argument may be an exact ref name (e.g. `refs/heads/release`) or a
> +glob pattern (e.g. `refs/heads/release/{asterisk}`).  The pattern syntax
> +is the same as for `--negotiation-restrict`.
> ++
> +If `--negotiation-restrict` is used, the have set is first restricted by
> +that option and then increased to include the tips specified by
> +`--negotiation-require`.

Very readable.  Nice.

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 57b2b667ff..b60652e6b1 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -99,6 +99,7 @@ static struct transport *gsecondary;
>  static struct refspec refmap = REFSPEC_INIT_FETCH;
>  static struct string_list server_options = STRING_LIST_INIT_DUP;
>  static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
> +static struct string_list negotiation_require = STRING_LIST_INIT_NODUP;

I thought _tip was renamed to _restrict in an earlier step, but that
was only in the transport in [3/7].  Perhaps we want to rename the
file-scope static variable negotiation_tip to negotiation_restrict
in an earlier step, like in [2/7]?

> +	for_each_string_list_item(item, negotiation_require) {
> +		if (!has_glob_specials(item->string)) {
> +			struct object_id oid;
> +			if (repo_get_oid(the_repository, item->string, &oid))
> +				continue;

The configuration (or command line) says --nego-require=refs/heads/main
but this old repository only has refs/heads/master; we do not want
to error out in such a case.

Is it true, though?  nego-{require,restrict} feels quite tied to
each project and unless the configuration or command line options
are applied blindly regardless of the project, such an error should
not happen.  Perhaps the user who gives a command line option
"--nego-require=refs/heads/naster" may want to be reminded of a
possible typo?

> +			if (!odb_has_object(the_repository->objects, &oid, 0))
> +				continue;

This is a bit curious.  When does the first condition holds but not
the second?  A lazy clone whose ref-tip contains a missing commit
promised by somebody else?

In the presense of "promised objects are allowed to be missing"
rule, silently skipping a missing object here is certainly
conservative, but this is not an object that is buried deep in a
tree hierarchy, but the top-level commit or tag that is directly
pointed at by a ref, isn't it?  I am a bit uneasy that we ignore
such potential repository corruption (i.e., a missing object may not
be something a promisor remote promised but simply missing).

> @@ -474,7 +511,25 @@ static int find_common(struct fetch_negotiator *negotiator,
>  	trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository);
>  	flushes = 0;
>  	retval = -1;
> +
> +	/* Send unconditional haves from --negotiation-require */
> +	resolve_negotiation_require(args->negotiation_require,
> +				    &negotiation_require_oids);
> +	if (oidset_size(&negotiation_require_oids)) {
> +		struct oidset_iter iter;
> +		oidset_iter_init(&negotiation_require_oids, &iter);
> +
> +		while ((oid = oidset_iter_next(&iter))) {
> +			packet_buf_write(&req_buf, "have %s\n",
> +					 oid_to_hex(oid));
> +			print_verbose(args, "have %s", oid_to_hex(oid));
> +		}
> +	}

OK.  I think it makes sense to send these early.  We have already
dealt with the usual "tips" by calling mark_tips() way earlier, but
that hasn't produced any "have" yet, and these will go before the
ones from traversal.  We do not traverse from these "require" and
that may be why these are not called "_tips"?

And sending these early means the other side has much less chance to
say "we've heard enough, stop!", so in a sense they are of much
higher priority "have"s (I wonder what happens when they do want to
say "stop!" while we are giving a lot of "have" from this loop,
though).

>  	while ((oid = negotiator->next(negotiator))) {
> +		/* avoid duplicate oids from --negotiation-require */
> +		if (oidset_contains(&negotiation_require_oids, oid))
> +			continue;

If objects rechable from "require" are traversed like others, then
this "avoid duplicate" would become unnecessary, right?

On 4/15/26 3:50 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <[email protected]> writes:
...
>> +static struct string_list negotiation_require = STRING_LIST_INIT_NODUP;
> > I thought _tip was renamed to _restrict in an earlier step, but that
> was only in the transport in [3/7].  Perhaps we want to rename the
> file-scope static variable negotiation_tip to negotiation_restrict
> in an earlier step, like in [2/7]?

This one was missed but will be fixed in v3.

>> +	for_each_string_list_item(item, negotiation_require) {
>> +		if (!has_glob_specials(item->string)) {
>> +			struct object_id oid;
>> +			if (repo_get_oid(the_repository, item->string, &oid))
>> +				continue;
> > The configuration (or command line) says --nego-require=refs/heads/main
> but this old repository only has refs/heads/master; we do not want
> to error out in such a case.
> > Is it true, though?  nego-{require,restrict} feels quite tied to
> each project and unless the configuration or command line options
> are applied blindly regardless of the project, such an error should
> not happen.  Perhaps the user who gives a command line option
> "--nego-require=refs/heads/naster" may want to be reminded of a
> possible typo?

You're right here. We shouldn't die() on a bad ref passed this way.

This should be a best-effort attempt to include a "have" and continue
normally if it isn't found.

>> +			if (!odb_has_object(the_repository->objects, &oid, 0))
>> +				continue;
> > This is a bit curious.  When does the first condition holds but not
> the second?  A lazy clone whose ref-tip contains a missing commit
> promised by somebody else?

Good point. This would occur if a ref exists but points to a missing
object, which should mean the repo is corrupt and we can't trust that
the fetch will succeed (or should).

> In the presense of "promised objects are allowed to be missing"
> rule, silently skipping a missing object here is certainly
> conservative, but this is not an object that is buried deep in a
> tree hierarchy, but the top-level commit or tag that is directly
> pointed at by a ref, isn't it?  I am a bit uneasy that we ignore
> such potential repository corruption (i.e., a missing object may not
> be something a promisor remote promised but simply missing).

I'll update this to be an error.

>> @@ -474,7 +511,25 @@ static int find_common(struct fetch_negotiator *negotiator,
>>   	trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository);
>>   	flushes = 0;
>>   	retval = -1;
>> +
>> +	/* Send unconditional haves from --negotiation-require */
>> +	resolve_negotiation_require(args->negotiation_require,
>> +				    &negotiation_require_oids);
>> +	if (oidset_size(&negotiation_require_oids)) {
>> +		struct oidset_iter iter;
>> +		oidset_iter_init(&negotiation_require_oids, &iter);
>> +
>> +		while ((oid = oidset_iter_next(&iter))) {
>> +			packet_buf_write(&req_buf, "have %s\n",
>> +					 oid_to_hex(oid));
>> +			print_verbose(args, "have %s", oid_to_hex(oid));
>> +		}
>> +	}
> > OK.  I think it makes sense to send these early.  We have already
> dealt with the usual "tips" by calling mark_tips() way earlier, but
> that hasn't produced any "have" yet, and these will go before the
> ones from traversal.  We do not traverse from these "require" and
> that may be why these are not called "_tips"?

It is correct that these required haves are not plugged into the
walk.

> And sending these early means the other side has much less chance to
> say "we've heard enough, stop!", so in a sense they are of much
> higher priority "have"s (I wonder what happens when they do want to
> say "stop!" while we are giving a lot of "have" from this loop,
> though).

I believe that we don't give the server an opportunity to say "stop"
until we've completed a "round" (see the 'if (flush_at <= ++count)'
case).

With this in mind, I should be incrementing 'count' while sending
the required haves.

>>   	while ((oid = negotiator->next(negotiator))) {
>> +		/* avoid duplicate oids from --negotiation-require */
>> +		if (oidset_contains(&negotiation_require_oids, oid))
>> +			continue;
> > If objects rechable from "require" are traversed like others, then
> this "avoid duplicate" would become unnecessary, right?

Yes, if we add the required things to the traversal, then we wouldn't
worry about duplicates. We'd also need to do things in a different
way:

1. The negotiator has a next() method that could do a number of
   things, but is responsible for walking history and ignoring IDs
   that are reachable from already-emitted haves.

2. This _could_ help us avoid sending required ID if we have
   emitted a have that could reach that ID.

3. However, this requires waiting until we flush a round of haves
   before determining if we should send the required IDs based on
   the walk to this point.

Perhaps the better way to incorporate things together would be to
mark the required IDs as COMMON as we emit them, which would signal
to the negotiation walks that they should not re-emit it as a have
(but since we don't add SEEN, they still walk through the commit to
its later ancestors).

Thanks,
-Stolee

"Derrick Stolee via GitGitGadget" <[email protected]> writes:

> -			warning("ignoring --negotiation-tip=%s because it does not match any refs",
> -				s);
> +			warning(_("ignoring %s=%s because it does not match any refs"),
> +				"--negotiation-restrict", s);
> -			warning("ignoring --negotiation-tip because the protocol does not support it");
> +			warning(_("ignoring %s because the protocol does not support it"),
> +				"--negotiation-restrict");

These are nice touches to make sure translators cannot possibly
botch these option names that must be given verbatim.

>  	}
>  	return transport;
>  }
> @@ -2567,6 +2568,8 @@ int cmd_fetch(int argc,
>  		OPT_IPVERSION(&family),
>  		OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
>  				N_("report that we have only objects reachable from this object")),
> +		OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
> +				N_("report that we have only objects reachable from this object")),

Is OPT_ALIAS() suitable for this?

> @@ -2657,7 +2660,7 @@ int cmd_fetch(int argc,
>  	}
>  
>  	if (negotiate_only && !negotiation_tip.nr)
> -		die(_("--negotiate-only needs one or more --negotiation-tip=*"));
> +		die(_("--negotiate-only needs one or more --negotiation-restrict=*"));

OK.  Shouldn't this also do the "%s" thing?

On 4/15/26 5:57 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <[email protected]> writes:
> >> -			warning("ignoring --negotiation-tip=%s because it does not match any refs",
>> -				s);
>> +			warning(_("ignoring %s=%s because it does not match any refs"),
>> +				"--negotiation-restrict", s);
>> -			warning("ignoring --negotiation-tip because the protocol does not support it");
>> +			warning(_("ignoring %s because the protocol does not support it"),
>> +				"--negotiation-restrict");
> > These are nice touches to make sure translators cannot possibly
> botch these option names that must be given verbatim.

>> @@ -2657,7 +2660,7 @@ int cmd_fetch(int argc,
>>   	}
>>
>>   	if (negotiate_only && !negotiation_tip.nr)
>> -		die(_("--negotiate-only needs one or more --negotiation-tip=*"));
>> +		die(_("--negotiate-only needs one or more --negotiation-restrict=*"));
>
> OK.  Shouldn't this also do the "%s" thing?

I think I had focused on adding "%s" to strings that were not
previously translated, but adjusting the string under translation
is enough to require retranslation. I should make it easier to
translate, too.

>>   	}
>>   	return transport;
>>   }
>> @@ -2567,6 +2568,8 @@ int cmd_fetch(int argc,
>>   		OPT_IPVERSION(&family),
>>   		OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
>>   				N_("report that we have only objects reachable from this object")),
>> +		OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
>> +				N_("report that we have only objects reachable from this object")),
> > Is OPT_ALIAS() suitable for this?

I was not aware of this. Thanks for the pointer!

I do plan to make "negotiation-tip" an alias for "negotiation-restrict"
based on the new preference for *-restrict as the "real" option now. Is
that the right way to do this?

Thanks,
-Stolee

Derrick Stolee <[email protected]> writes:

>>>   		OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
>>>   				N_("report that we have only objects reachable from this object")),
>>> +		OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
>>> +				N_("report that we have only objects reachable from this object")),
>> 
>> Is OPT_ALIAS() suitable for this?
>
> I was not aware of this. Thanks for the pointer!
>
> I do plan to make "negotiation-tip" an alias for "negotiation-restrict"
> based on the new preference for *-restrict as the "real" option now. Is
> that the right way to do this?

Let's see.

$ git grep OPT_ALIAS builtin/clone.c
builtin/clone.c:		OPT_ALIAS(0, "recursive", "recurse-submodules"),
$ git clone -h
usage: git clone [<options>] [--] <repo> [<dir>]

    -v, --[no-]verbose    be more verbose
    -q, --[no-]quiet      be more quiet
    ...
    --[no-]recurse-submodules[=<pathspec>]
                          initialize submodules in the clone
    --[no-]recursive[=<pathspec>]
                          alias of --recurse-submodules
    ...

I think we gave the operation the name "recursive", with a common
short sightedness that anything we are adding "recursive" for is the
only kind of recursiveness, and then prepared for a future where
things other than submodules can also be sources of recursiveness by
making "recurse-submodules" the official name, while still allowing
historical name as the synonym.

In this case, if "-restrict" will become the official name, it
should be listed first, and then the historical name should be made
its alias.

So

	OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, ...),
	OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),

would be the right combination in the correct order, I think.
Mention the official thing first, and then tell that another thing
is an alias to what the readers have already seen after that (e.g.,
c28b036f (clone: reorder --recursive/--recurse-submodules,
2020-03-16)).

On 4/20/2026 6:32 AM, Junio C Hamano wrote:
> Derrick Stolee <[email protected]> writes:
> 
>>>>   		OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
>>>>   				N_("report that we have only objects reachable from this object")),
>>>> +		OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
>>>> +				N_("report that we have only objects reachable from this object")),
>>>
>>> Is OPT_ALIAS() suitable for this?
>>
>> I was not aware of this. Thanks for the pointer!
>>
>> I do plan to make "negotiation-tip" an alias for "negotiation-restrict"
>> based on the new preference for *-restrict as the "real" option now. Is
>> that the right way to do this?
> 
> Let's see.
...
> So
> 
> 	OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, ...),
> 	OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
> 
> would be the right combination in the correct order, I think.
> Mention the official thing first, and then tell that another thing
> is an alias to what the readers have already seen after that (e.g.,
> c28b036f (clone: reorder --recursive/--recurse-submodules,
> 2020-03-16)).

Thanks! This is indeed what I have in my local copy in preparation
for v3. It helps to have early confirmation about this.

-Stolee

On Wed, Apr 15, 2026 at 03:14:22PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 3bcb0c9686..4c3c5f2faa 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c

Don't we want to also rename the local `negotiation_tip` variable in
`cmd_fetch()`?

Patrick

On Wed, Apr 15, 2026 at 03:14:24PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <[email protected]>
> 
> Add a new --negotiation-require option to 'git fetch', which ensures
> that certain ref tips are always sent as 'have' lines during fetch
> negotiation, regardless of what the negotiation algorithm selects.

When reading "--negotiation-require" my mind immediately shifts towards
a mode where we require the remote to have a specific reference, and if
not we'll abort. That's of course not what you're proposing here, but I
would think that I may not be the only person making that connection.

Would an alternative like "--negotiation-include" or
"--negotiation-expand" be better?

> diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
> index c07b85499f..85ffc5b32b 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -73,6 +73,25 @@ See also the `fetch.negotiationAlgorithm` and `push.negotiate`
>  configuration variables documented in linkgit:git-config[1], and the
>  `--negotiate-only` option below.
>  
> +`--negotiation-require=<revision>`::
> +	Ensure that the given ref tip is always sent as a "have" line
> +	during fetch negotiation, regardless of what the negotiation
> +	algorithm selects.  This is useful to guarantee that common
> +	history reachable from specific refs is always considered, even
> +	when `--negotiation-restrict` restricts the set of tips or when
> +	the negotiation algorithm would otherwise skip them.
> ++
> +This option may be specified more than once; if so, each ref is sent
> +unconditionally.
> ++
> +The argument may be an exact ref name (e.g. `refs/heads/release`) or a
> +glob pattern (e.g. `refs/heads/release/{asterisk}`).  The pattern syntax
> +is the same as for `--negotiation-restrict`.
> ++
> +If `--negotiation-restrict` is used, the have set is first restricted by
> +that option and then increased to include the tips specified by
> +`--negotiation-require`.

This interaction makes sense. You can basically say "send only local
branches, but please _also_ send that one particular ref over there".

> diff --git a/fetch-pack.c b/fetch-pack.c
> index baf239adf9..a0029253f1 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -474,7 +511,25 @@ static int find_common(struct fetch_negotiator *negotiator,
>  	trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository);
>  	flushes = 0;
>  	retval = -1;
> +
> +	/* Send unconditional haves from --negotiation-require */
> +	resolve_negotiation_require(args->negotiation_require,
> +				    &negotiation_require_oids);
> +	if (oidset_size(&negotiation_require_oids)) {
> +		struct oidset_iter iter;
> +		oidset_iter_init(&negotiation_require_oids, &iter);
> +
> +		while ((oid = oidset_iter_next(&iter))) {
> +			packet_buf_write(&req_buf, "have %s\n",
> +					 oid_to_hex(oid));
> +			print_verbose(args, "have %s", oid_to_hex(oid));
> +		}
> +	}

Okay, so here we now unconditionally send our requested object IDs.

One thing I was wondering is whether we need to flush eventually. It can
happen that the user specifies millions of refs, either intentionally or
by accident. But I guess the answer might be "no", as the intent of the
feature is that we indeed want to send all of those to the remote side,
and the remote is being asked to consider all of those OIDs.

Patrick

On 4/20/2026 4:11 AM, Patrick Steinhardt wrote:
> On Wed, Apr 15, 2026 at 03:14:24PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <[email protected]>
>>
>> Add a new --negotiation-require option to 'git fetch', which ensures
>> that certain ref tips are always sent as 'have' lines during fetch
>> negotiation, regardless of what the negotiation algorithm selects.
> 
> When reading "--negotiation-require" my mind immediately shifts towards
> a mode where we require the remote to have a specific reference, and if
> not we'll abort. That's of course not what you're proposing here, but I
> would think that I may not be the only person making that connection.
> 
> Would an alternative like "--negotiation-include" or
> "--negotiation-expand" be better?

"include" does sound good to me. I'm open to it. I'll let this idea
stew and try prepping my local branch in this direction.
 
>> +	/* Send unconditional haves from --negotiation-require */
>> +	resolve_negotiation_require(args->negotiation_require,
>> +				    &negotiation_require_oids);
>> +	if (oidset_size(&negotiation_require_oids)) {
>> +		struct oidset_iter iter;
>> +		oidset_iter_init(&negotiation_require_oids, &iter);
>> +
>> +		while ((oid = oidset_iter_next(&iter))) {
>> +			packet_buf_write(&req_buf, "have %s\n",
>> +					 oid_to_hex(oid));
>> +			print_verbose(args, "have %s", oid_to_hex(oid));
>> +		}
>> +	}
> 
> Okay, so here we now unconditionally send our requested object IDs.
> 
> One thing I was wondering is whether we need to flush eventually. It can
> happen that the user specifies millions of refs, either intentionally or
> by accident. But I guess the answer might be "no", as the intent of the
> feature is that we indeed want to send all of those to the remote side,
> and the remote is being asked to consider all of those OIDs.

The idea is indeed to send all of the requested OIDs, but this does
present an interesting behavior where the Git client can allow the
user to misconfigure themselves to send larger-than-normal negotiation
requests. Previously, the client would protect the negotiation with a
maximum set of haves.

Is there any concern about this becoming a vector for increased load
on servers?

Would it be good to have some kind of advice message when the config
matches a set of haves that we think is too large? That would maybe
be a way to help users get out of a self-made problem.

Thanks,
-Stolee

On 2026-05-14 13:41, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <[email protected]>
> > The 'fetch follows tags by default' test sorts using 'sort -k 4', but
> for-each-ref output only has 3 columns. This relies on sort treating records
> with fewer fields as having an empty fourth field, which may produce
> unstable results depending on locale. This appears to be an accident added
> in 3f763ddf28 (fetch: set remote/HEAD if it does not exist, 2024-11-22).
> > Use 'sort -k 3' to match the actual number of columns in the output.

Expanding the message to back reference 3f763ddf28 is a nice change.
Makes it easier for future people reading the history to follow back.

> Signed-off-by: Derrick Stolee <[email protected]>
> ---
>   t/t5516-fetch-push.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> > diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 29e2f17608..ac8447f21e 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1349,7 +1349,7 @@ test_expect_success 'fetch follows tags by default' '
>   		git for-each-ref >tmp1 &&
>   		sed -n "p; s|refs/heads/main$|refs/remotes/origin/main|p" tmp1 |
>   		sed -n "p; s|refs/heads/main$|refs/remotes/origin/HEAD|p"  |
> -		sort -k 4 >../expect
> +		sort -k 3 >../expect
>   	) &&
>   	test_when_finished "rm -rf dst" &&
>   	git init dst &&

Unchanged from v3 and still LGTM!

Thanks,
Matthew

On 2026-05-14 13:41, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <[email protected]>
> > The --negotiation-tip option to 'git fetch' and 'git pull' allows users
> to specify that they want to focus negotiation on a small set of
> references. This is a _restriction_ on the negotiation set, helping to
> focus the negotiation when the ref count is high. However, it doesn't
> allow for the ability to opportunistically select references beyond that
> list.
> > This subtle detail that this is a 'maximum set' and not a 'minimum set'
> is not immediately clear from the option name. This makes it more
> complicated to add a new option that provides the complementary behavior
> of a minimum set.
> > For now, create a new synonym option, --negotiation-restrict, that
> behaves identically to --negotiation-tip. Update the documentation to
> make it clear that this new name is the preferred option, but we keep
> the old name for compatibility. Mark --negotiation-tip as an alias of the
> new, preferred option.
> > Update a few warning messages with the new option, but also make them
> translatable with the option name inserted by formatting. At least one
> of these messages will be reused later for a new option.
> > Signed-off-by: Derrick Stolee <[email protected]>
> ---
>   Documentation/config/fetch.adoc  |  2 +-
>   Documentation/fetch-options.adoc |  6 +++++-
>   builtin/fetch.c                  | 13 ++++++++-----
>   builtin/pull.c                   |  3 ++-
>   send-pack.c                      |  2 +-
>   t/t5510-fetch.sh                 | 25 +++++++++++++++++++++++++
>   t/t5702-protocol-v2.sh           |  4 ++--
>   transport-helper.c               |  3 ++-
>   8 files changed, 46 insertions(+), 12 deletions(-)
> > diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
> index cd40db0cad..04ac90912d 100644
> --- a/Documentation/config/fetch.adoc
> +++ b/Documentation/config/fetch.adoc
> @@ -76,7 +76,7 @@
>   	default is `skipping`.  Unknown values will cause `git fetch` to
>   	error out.
>   +
> -See also the `--negotiate-only` and `--negotiation-tip` options to
> +See also the `--negotiate-only` and `--negotiation-restrict` options to
>   linkgit:git-fetch[1].

Good - this addressed my nit in v4 about mentioning the old name here.

>   `fetch.showForcedUpdates`::
> diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
> index 81a9d7f9bb..d39cecb446 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -49,6 +49,7 @@ the current repository has the same history as the source repository.
>   	`.git/shallow`. This option updates `.git/shallow` and accepts such
>   	refs.
>   > +`--negotiation-restrict=(<commit>|<glob>)`::
>   `--negotiation-tip=(<commit>|<glob>)`::
>   	By default, Git will report, to the server, commits reachable
>   	from all local refs to find common commits in an attempt to
> @@ -58,6 +59,9 @@ the current repository has the same history as the source repository.
>   	local ref is likely to have commits in common with the
>   	upstream ref being fetched.
>   +
> +`--negotiation-restrict` is the preferred name for this option;
> +`--negotiation-tip` is accepted as a synonym.
> ++
>   This option may be specified more than once; if so, Git will report
>   commits reachable from any of the given commits.
>   +
> @@ -71,7 +75,7 @@ configuration variables documented in linkgit:git-config[1], and the
>   >   `--negotiate-only`::
>   	Do not fetch anything from the server, and instead print the
> -	ancestors of the provided `--negotiation-tip=` arguments,
> +	ancestors of the provided `--negotiation-restrict=` arguments,
>   	which we have in common with the server.
>   +
>   This is incompatible with `--recurse-submodules=(yes|on-demand)`.

Good! The --negotiate-only paragraph now uses --negotiate-restrict too.

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 4795b2a13c..fc950fe35b 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1558,8 +1558,8 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
>   		refs_for_each_ref_ext(get_main_ref_store(the_repository),
>   				      add_oid, oids, &opts);
>   		if (old_nr == oids->nr)
> -			warning("ignoring --negotiation-tip=%s because it does not match any refs",
> -				s);
> +			warning(_("ignoring %s=%s because it does not match any refs"),
> +				"--negotiation-restrict", s);
>   	}
>   	smart_options->negotiation_tips = oids;
>   }
> @@ -1599,7 +1599,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
>   		if (transport->smart_options)
>   			add_negotiation_tips(transport->smart_options);
>   		else
> -			warning("ignoring --negotiation-tip because the protocol does not support it");
> +			warning(_("ignoring %s because the protocol does not support it"),
> +				"--negotiation-restrict");
>   	}
>   	return transport;
>   }
> @@ -2565,8 +2566,9 @@ int cmd_fetch(int argc,
>   			       N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
>   		OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
>   		OPT_IPVERSION(&family),
> -		OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
> +		OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
>   				N_("report that we have only objects reachable from this object")),
> +		OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
>   		OPT_BOOL(0, "negotiate-only", &negotiate_only,
>   			 N_("do not fetch a packfile; instead, print ancestors of negotiation tips")),
>   		OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
> @@ -2657,7 +2659,8 @@ int cmd_fetch(int argc,
>   	}
>   >   	if (negotiate_only && !negotiation_tip.nr)
> -		die(_("--negotiate-only needs one or more --negotiation-tip=*"));
> +		die(_("%s needs one or more %s"), "--negotiate-only",
> +		    "--negotiation-restrict=*");
>   >   	if (deepen_relative) {
>   		if (deepen_relative < 0)

Unchanged from v3: OPT_ALIAS keeps back-compat and messages are i18n
friendly. Nice.

> diff --git a/builtin/pull.c b/builtin/pull.c
> index 7e67fdce97..cc6ce485fc 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -996,9 +996,10 @@ int cmd_pull(int argc,
>   		OPT_PASSTHRU('6',  "ipv6", &opt_ipv6, NULL,
>   			N_("use IPv6 addresses only"),
>   			PARSE_OPT_NOARG),
> -		OPT_PASSTHRU_ARGV(0, "negotiation-tip", &opt_fetch, N_("revision"),
> +		OPT_PASSTHRU_ARGV(0, "negotiation-restrict", &opt_fetch, N_("revision"),
>   			N_("report that we have only objects reachable from this object"),
>   			0),
> +		OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
>   		OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
>   			 N_("check for forced-updates on all updated branches")),
>   		OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,

Oh nice! v3 had two OPT_PASSTHRU_ARGV (one for each name). Now we're
using OPT_ALIAS here too for the old name which avoids duplicated
description strings - yay!

I had to convince myself this works (does OPT_ALIAS forward to the
passthru option?). Looking at parse-options.c, preprocess_options()
substitutes the alias with a copy of the source option keeping only
the alias's long_name. So really the child proc will get the old
--negotiate-tip name but the child has an alias anyway.. works fine.

> diff --git a/send-pack.c b/send-pack.c
> index 67d6987b1c..3d5d36ba3b 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -447,7 +447,7 @@ static void get_commons_through_negotiation(struct repository *r,
>   	strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
>   	for (ref = remote_refs; ref; ref = ref->next) {
>   		if (!is_null_oid(&ref->new_oid)) {
> -			strvec_pushf(&child.args, "--negotiation-tip=%s",
> +			strvec_pushf(&child.args, "--negotiation-restrict=%s",
>   				     oid_to_hex(&ref->new_oid));
>   			nr_negotiation_tip++;
>   		}

Good. v3 was using the old name when shelling out (just a nit), but
nicer to have the rename fully consistent.

> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index 5dcb4b51a4..dc3ce56d84 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -1460,6 +1460,31 @@ EOF
>   	test_cmp fatal-expect fatal-actual
>   '
>   > +test_expect_success '--negotiation-restrict limits "have" lines sent' '
> +	setup_negotiation_tip server server 0 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 --negotiation-restrict=beta_1 \
> +		origin alpha_s beta_s &&
> +	check_negotiation_tip
> +'
> +
> +test_expect_success '--negotiation-restrict understands globs' '
> +	setup_negotiation_tip server server 0 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=*_1 \
> +		origin alpha_s beta_s &&
> +	check_negotiation_tip
> +'
> +
> +test_expect_success '--negotiation-restrict and --negotiation-tip can be mixed' '
> +	setup_negotiation_tip server server 0 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		--negotiation-tip=beta_1 \
> +		origin alpha_s beta_s &&
> +	check_negotiation_tip
> +'
> +
>   test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
>   	git init df-conflict &&
>   	(
> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
> index f826ac46a5..9f6cf4142d 100755
> --- a/t/t5702-protocol-v2.sh
> +++ b/t/t5702-protocol-v2.sh
> @@ -869,14 +869,14 @@ setup_negotiate_only () {
>   	test_commit -C client three
>   }
>   > -test_expect_success 'usage: --negotiate-only without --negotiation-tip' '
> +test_expect_success 'usage: --negotiate-only without --negotiation-restrict' '
>   	SERVER="server" &&
>   	URI="file://$(pwd)/server" &&
>   >   	setup_negotiate_only "$SERVER" "$URI" &&
>   >   	cat >err.expect <<-\EOF &&
> -	fatal: --negotiate-only needs one or more --negotiation-tip=*
> +	fatal: --negotiate-only needs one or more --negotiation-restrict=*
>   	EOF
>   >   	test_must_fail git -c protocol.version=2 -C client fetch \

Unchanged from v3 - still good!

> diff --git a/transport-helper.c b/transport-helper.c
> index 4d95d84f9e..dd78d40668 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -755,7 +755,8 @@ static int fetch_refs(struct transport *transport,
>   	}
>   >   	if (data->transport_options.negotiation_tips)
> -		warning("Ignoring --negotiation-tip because the protocol does not support it.");
> +		warning(_("ignoring %s because the protocol does not support it."),
> +			"--negotiation-restrict");

Good - picks up a missed rename from v3.

>   	if (data->fetch)
>   		return fetch_with_fetch(transport, nr_heads, to_fetch);

Overall this patch LGTM! All my v3 issues were addressed.

Thanks,
Matthew

On 2026-05-14 13:41, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <[email protected]>
> > The previous change added the --negotiation-restrict synonym for the
> --negotiation-tip option for 'git fetch'. In anticipation of adding a new
> option that behaves similarly but with distinct changes to its behavior,
> rename the internal representation of this data from 'negotiation_tips' to
> 'negotiation_restrict_tips'.

The typo (plural vs singular 'tips') is fixed - thanks!

> The 'tips' part is kept because this is an oid_array in the transport layer.
> This requires the builtin to handle parsing refs into collections of oids so
> the transport layer can handle this cleaner form of the data.
> > Also update the string_list used to store the inputs from command-line
> options.
> > Signed-off-by: Derrick Stolee <[email protected]>
> ---
>   builtin/fetch.c    | 18 +++++++++---------
>   fetch-pack.c       | 18 +++++++++---------
>   fetch-pack.h       |  4 ++--
>   transport-helper.c |  2 +-
>   transport.c        | 10 +++++-----
>   transport.h        |  4 ++--
>   6 files changed, 28 insertions(+), 28 deletions(-)
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> index fc950fe35b..2ba0051d52 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -98,7 +98,7 @@ static struct transport *gtransport;
>   static struct transport *gsecondary;
>   static struct refspec refmap = REFSPEC_INIT_FETCH;
>   static struct string_list server_options = STRING_LIST_INIT_DUP;
> -static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
> +static struct string_list negotiation_restrict = STRING_LIST_INIT_NODUP;
>   >   struct fetch_config {
>   	enum display_format display_format;
> @@ -1534,13 +1534,13 @@ static int add_oid(const struct reference *ref, void *cb_data)
>   	return 0;
>   }
>   > -static void add_negotiation_tips(struct git_transport_options *smart_options)
> +static void add_negotiation_restrict_tips(struct git_transport_options *smart_options)
>   {
>   	struct oid_array *oids = xcalloc(1, sizeof(*oids));
>   	int i;
>   > -	for (i = 0; i < negotiation_tip.nr; i++) {
> -		const char *s = negotiation_tip.items[i].string;
> +	for (i = 0; i < negotiation_restrict.nr; i++) {
> +		const char *s = negotiation_restrict.items[i].string;
>   		struct refs_for_each_ref_options opts = {
>   			.pattern = s,
>   		};
> @@ -1561,7 +1561,7 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
>   			warning(_("ignoring %s=%s because it does not match any refs"),
>   				"--negotiation-restrict", s);
>   	}
> -	smart_options->negotiation_tips = oids;
> +	smart_options->negotiation_restrict_tips = oids;
>   }

Same as in v3, this function will be refactored in a later patch, so
just doing the rename now keeps things reviewable - thanks!

>   static struct transport *prepare_transport(struct remote *remote, int deepen,
> @@ -1595,9 +1595,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
>   		set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER, spec);
>   		set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
>   	}
> -	if (negotiation_tip.nr) {
> +	if (negotiation_restrict.nr) {
>   		if (transport->smart_options)
> -			add_negotiation_tips(transport->smart_options);
> +			add_negotiation_restrict_tips(transport->smart_options);
>   		else
>   			warning(_("ignoring %s because the protocol does not support it"),
>   				"--negotiation-restrict");
> @@ -2566,7 +2566,7 @@ int cmd_fetch(int argc,
>   			       N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
>   		OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
>   		OPT_IPVERSION(&family),
> -		OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
> +		OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_restrict, N_("revision"),
>   				N_("report that we have only objects reachable from this object")),
>   		OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
>   		OPT_BOOL(0, "negotiate-only", &negotiate_only,
> @@ -2658,7 +2658,7 @@ int cmd_fetch(int argc,
>   		config.display_format = DISPLAY_FORMAT_PORCELAIN;
>   	}
>   > -	if (negotiate_only && !negotiation_tip.nr)
> +	if (negotiate_only && !negotiation_restrict.nr)
>   		die(_("%s needs one or more %s"), "--negotiate-only",
>   		    "--negotiation-restrict=*");

Simple rename - looks good!

> diff --git a/fetch-pack.c b/fetch-pack.c
> index 6ecd468ef7..baf239adf9 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -291,21 +291,21 @@ static int next_flush(int stateless_rpc, int count)
>   }
>   >   static void mark_tips(struct fetch_negotiator *negotiator,
> -		      const struct oid_array *negotiation_tips)
> +		      const struct oid_array *negotiation_restrict_tips)
>   {
>   	struct refs_for_each_ref_options opts = {
>   		.flags = REFS_FOR_EACH_INCLUDE_BROKEN,
>   	};
>   	int i;
>   > -	if (!negotiation_tips) {
> +	if (!negotiation_restrict_tips) {
>   		refs_for_each_ref_ext(get_main_ref_store(the_repository),
>   				      rev_list_insert_ref_oid, negotiator, &opts);
>   		return;
>   	}
>   > -	for (i = 0; i < negotiation_tips->nr; i++)
> -		rev_list_insert_ref(negotiator, &negotiation_tips->oid[i]);
> +	for (i = 0; i < negotiation_restrict_tips->nr; i++)
> +		rev_list_insert_ref(negotiator, &negotiation_restrict_tips->oid[i]);
>   	return;
>   }
>   > @@ -355,7 +355,7 @@ static int find_common(struct fetch_negotiator *negotiator,
>   			   PACKET_READ_CHOMP_NEWLINE |
>   			   PACKET_READ_DIE_ON_ERR_PACKET);
>   > -	mark_tips(negotiator, args->negotiation_tips);
> +	mark_tips(negotiator, args->negotiation_restrict_tips);
>   	for_each_cached_alternate(negotiator, insert_one_alternate_object);
>   >   	fetching = 0;
> @@ -1728,7 +1728,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
>   			else
>   				state = FETCH_SEND_REQUEST;
>   > -			mark_tips(negotiator, args->negotiation_tips);
> +			mark_tips(negotiator, args->negotiation_restrict_tips);
>   			for_each_cached_alternate(negotiator,
>   						  insert_one_alternate_object);
>   			break;
> @@ -2177,7 +2177,7 @@ static void clear_common_flag(struct oidset *s)
>   	}
>   }
>   > -void negotiate_using_fetch(const struct oid_array *negotiation_tips,
> +void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
>   			   const struct string_list *server_options,
>   			   int stateless_rpc,
>   			   int fd[],
> @@ -2195,13 +2195,13 @@ void negotiate_using_fetch(const struct oid_array *negotiation_tips,
>   	timestamp_t min_generation = GENERATION_NUMBER_INFINITY;
>   >   	fetch_negotiator_init(the_repository, &negotiator);
> -	mark_tips(&negotiator, negotiation_tips);
> +	mark_tips(&negotiator, negotiation_restrict_tips);
>   >   	packet_reader_init(&reader, fd[0], NULL, 0,
>   			   PACKET_READ_CHOMP_NEWLINE |
>   			   PACKET_READ_DIE_ON_ERR_PACKET);
>   > -	oid_array_for_each((struct oid_array *) negotiation_tips,
> +	oid_array_for_each((struct oid_array *) negotiation_restrict_tips,
>   			   add_to_object_array,
>   			   &nt_object_array);
>   > diff --git a/fetch-pack.h b/fetch-pack.h
> index 9d3470366f..6c70c942c2 100644
> --- a/fetch-pack.h
> +++ b/fetch-pack.h
> @@ -21,7 +21,7 @@ struct fetch_pack_args {
>   	 * If not NULL, during packfile negotiation, fetch-pack will send "have"
>   	 * lines only with these tips and their ancestors.
>   	 */
> -	const struct oid_array *negotiation_tips;
> +	const struct oid_array *negotiation_restrict_tips;
>   >   	unsigned deepen_relative:1;
>   	unsigned quiet:1;
> @@ -89,7 +89,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
>    * In the capability advertisement that has happened prior to invoking this
>    * function, the "wait-for-done" capability must be present.
>    */
> -void negotiate_using_fetch(const struct oid_array *negotiation_tips,
> +void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
>   			   const struct string_list *server_options,
>   			   int stateless_rpc,
>   			   int fd[],
> diff --git a/transport-helper.c b/transport-helper.c
> index dd78d40668..f4388da766 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -754,7 +754,7 @@ static int fetch_refs(struct transport *transport,
>   		set_helper_option(transport, "filter", spec);
>   	}
>   > -	if (data->transport_options.negotiation_tips)
> +	if (data->transport_options.negotiation_restrict_tips)
>   		warning(_("ignoring %s because the protocol does not support it."),
>   			"--negotiation-restrict");
>   > diff --git a/transport.c b/transport.c
> index 107f4fa5dc..a3051f6733 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -463,7 +463,7 @@ static int fetch_refs_via_pack(struct transport *transport,
>   	args.refetch = data->options.refetch;
>   	args.stateless_rpc = transport->stateless_rpc;
>   	args.server_options = transport->server_options;
> -	args.negotiation_tips = data->options.negotiation_tips;
> +	args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
>   	args.reject_shallow_remote = transport->smart_options->reject_shallow;
>   >   	if (!data->finished_handshake) {
> @@ -491,7 +491,7 @@ static int fetch_refs_via_pack(struct transport *transport,
>   			warning(_("server does not support wait-for-done"));
>   			ret = -1;
>   		} else {
> -			negotiate_using_fetch(data->options.negotiation_tips,
> +			negotiate_using_fetch(data->options.negotiation_restrict_tips,
>   					      transport->server_options,
>   					      transport->stateless_rpc,
>   					      data->fd,
> @@ -979,9 +979,9 @@ static int disconnect_git(struct transport *transport)
>   		finish_connect(data->conn);
>   	}
>   > -	if (data->options.negotiation_tips) {
> -		oid_array_clear(data->options.negotiation_tips);
> -		free(data->options.negotiation_tips);
> +	if (data->options.negotiation_restrict_tips) {
> +		oid_array_clear(data->options.negotiation_restrict_tips);
> +		free(data->options.negotiation_restrict_tips);
>   	}
>   	list_objects_filter_release(&data->options.filter_options);
>   	oid_array_clear(&data->extra_have);
> diff --git a/transport.h b/transport.h
> index 892f19454a..cdeb33c16f 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -40,13 +40,13 @@ struct git_transport_options {
>   >   	/*
>   	 * This is only used during fetch. See the documentation of
> -	 * negotiation_tips in struct fetch_pack_args.
> +	 * negotiation_restrict_tips in struct fetch_pack_args.
>   	 *
>   	 * This field is only supported by transports that support connect or
>   	 * stateless_connect. Set this field directly instead of using
>   	 * transport_set_option().
>   	 */
> -	struct oid_array *negotiation_tips;
> +	struct oid_array *negotiation_restrict_tips;
>   >   	/*
>   	 * If allocated, whenever transport_fetch_refs() is called, add known

All just simple renames - looks good to me! This patch is good.

Thanks,
Matthew

On 2026-05-14 13:41, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <[email protected]>
> > In a previous change, the --negotiation-restrict command-line option of 'git
> fetch' was added as a synonym of --negotiation-tip. Both of these options
> restrict the set of 'haves' the client can send as part of negotiation.
> > This was previously not available via a configuration option. Add a new
> 'remote.<name>.negotiationRestrict' multi-valued config option that updates
> 'git fetch <name>' to use these restrictions by default.
> > If the user provides even one --negotiation-restrict argument, then the
> config is ignored.
> > An empty value resets the value list to allow ignoring earlier config
> values, such as those that might be set in system or global config.
> > Signed-off-by: Derrick Stolee <[email protected]>
> ---
>   Documentation/config/remote.adoc | 18 ++++++++++++++++++
>   builtin/fetch.c                  | 28 +++++++++++++++++++++-------
>   remote.c                         |  5 +++++
>   remote.h                         |  1 +
>   t/t5510-fetch.sh                 | 26 ++++++++++++++++++++++++++
>   5 files changed, 71 insertions(+), 7 deletions(-)
> > diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
> index 91e46f66f5..4dcf81fbce 100644
> --- a/Documentation/config/remote.adoc
> +++ b/Documentation/config/remote.adoc
> @@ -107,6 +107,24 @@ priority configuration file (e.g. `.git/config` in a repository) to clear
>   the values inherited from a lower priority configuration files (e.g.
>   `$HOME/.gitconfig`).
>   > +remote.<name>.negotiationRestrict::
> +	When negotiating with this remote during `git fetch`, restrict the
> +	commits advertised as "have" lines to only those reachable from refs
> +	matching the given patterns.  This multi-valued config option behaves
> +	like `--negotiation-restrict` on the command line.
> ++
> +Each value is either an exact ref name (e.g. `refs/heads/release`) or a
> +glob pattern (e.g. `refs/heads/release/*`).  The pattern syntax is the
> +same as for `--negotiation-restrict`.
> ++
> +These config values are used as defaults for the `--negotiation-restrict`
> +command-line option.  If `--negotiation-restrict` (or its synonym
> +`--negotiation-tip`) is specified on the command line, then the config
> +values are not used.
> ++
> +Blank values signal to ignore all previous values, allowing a reset of
> +the list from broader config scenarios.
> +
>   remote.<name>.followRemoteHEAD::
>   	How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
>   	when fetching using the configured refspecs of a remote.

Good - now we don't talk about 'git push' before we have wired that
functionality up.

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 2ba0051d52..a957739f37 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1601,6 +1601,19 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
>   		else
>   			warning(_("ignoring %s because the protocol does not support it"),
>   				"--negotiation-restrict");
> +	} else if (remote->negotiation_restrict.nr) {
> +		struct string_list_item *item;
> +		for_each_string_list_item(item, &remote->negotiation_restrict)
> +			string_list_append(&negotiation_restrict, item->string);
> +		if (transport->smart_options)
> +			add_negotiation_restrict_tips(transport->smart_options);
> +		else {
> +			struct strbuf config_name = STRBUF_INIT;
> +			strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name);
> +			warning(_("ignoring %s because the protocol does not support it"),
> +				config_name.buf);
> +			strbuf_release(&config_name);
> +		}
>   	}
>   	return transport;
>   }

Unchanged from v3 and still reads cleanly. Good.

> @@ -2658,10 +2671,6 @@ int cmd_fetch(int argc,
>   		config.display_format = DISPLAY_FORMAT_PORCELAIN;
>   	}
>   > -	if (negotiate_only && !negotiation_restrict.nr)
> -		die(_("%s needs one or more %s"), "--negotiate-only",
> -		    "--negotiation-restrict=*");
> -
>   	if (deepen_relative) {
>   		if (deepen_relative < 0)
>   			die(_("negative depth in --deepen is not supported"));
> @@ -2749,14 +2758,19 @@ int cmd_fetch(int argc,
>   		if (!remote)
>   			die(_("must supply remote when using --negotiate-only"));
>   		gtransport = prepare_transport(remote, 1, &filter_options);
> -		if (gtransport->smart_options) {
> -			gtransport->smart_options->acked_commits = &acked_commits;
> -		} else {
> +
> +		if (!gtransport->smart_options) {
>   			warning(_("protocol does not support --negotiate-only, exiting"));
>   			result = 1;
>   			trace2_region_leave("fetch", "negotiate-only", the_repository);
>   			goto cleanup;
>   		}
> +		if (!gtransport->smart_options->negotiation_restrict_tips)
> +			die(_("%s needs one or more %s"), "--negotiate-only",
> +			    "--negotiation-restrict=*");
> +
> +		gtransport->smart_options->acked_commits = &acked_commits;
> +
>   		if (server_options.nr)
>   			gtransport->server_options = &server_options;
>   		result = transport_fetch_refs(gtransport, NULL);

This is nice! Addressed my concerns in v3 about the wrong message being
shown, and makes it clearer with an up-front check for the protocol
support.

> diff --git a/remote.c b/remote.c
> index 7ca2a6501b..620086e16e 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -152,6 +152,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
>   	refspec_init_push(&ret->push);
>   	refspec_init_fetch(&ret->fetch);
>   	string_list_init_dup(&ret->server_options);
> +	string_list_init_dup(&ret->negotiation_restrict);
>   >   	ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
>   		   remote_state->remotes_alloc);
> @@ -179,6 +180,7 @@ static void remote_clear(struct remote *remote)
>   	FREE_AND_NULL(remote->http_proxy);
>   	FREE_AND_NULL(remote->http_proxy_authmethod);
>   	string_list_clear(&remote->server_options, 0);
> +	string_list_clear(&remote->negotiation_restrict, 0);
>   }
>   >   static void add_merge(struct branch *branch, const char *name)
> @@ -562,6 +564,9 @@ static int handle_config(const char *key, const char *value,
>   	} else if (!strcmp(subkey, "serveroption")) {
>   		return parse_transport_option(key, value,
>   					      &remote->server_options);
> +	} else if (!strcmp(subkey, "negotiationrestrict")) {
> +		return parse_transport_option(key, value,
> +					      &remote->negotiation_restrict);
>   	} else if (!strcmp(subkey, "followremotehead")) {
>   		const char *no_warn_branch;
>   		if (!strcmp(value, "never"))

Good - reusing the parse_transport_option() as suggested makes this a
smaller diff and consistent with 'serveroption' above.

> diff --git a/remote.h b/remote.h
> index fc052945ee..e6ec37c393 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -117,6 +117,7 @@ struct remote {
>   	char *http_proxy_authmethod;
>   >   	struct string_list server_options;
> +	struct string_list negotiation_restrict;
>   >   	enum follow_remote_head_settings follow_remote_head;
>   	const char *no_warn_branch;
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index dc3ce56d84..eff3ce8e2d 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -1485,6 +1485,32 @@ test_expect_success '--negotiation-restrict and --negotiation-tip can be mixed'
>   	check_negotiation_tip
>   '
>   > +test_expect_success 'remote.<name>.negotiationRestrict used as default' '
> +	setup_negotiation_tip server server 0 &&
> +
> +	# test the reset of the list on an empty value
> +	git -C client config --add remote.origin.negotiationRestrict alpha_2 &&
> +	git -C client config --add remote.origin.negotiationRestrict "" &&
> +	git -C client config --add remote.origin.negotiationRestrict alpha_1 &&
> +	git -C client config --add remote.origin.negotiationRestrict beta_1 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		origin alpha_s beta_s &&
> +	check_negotiation_tip
> +'
> +
> +test_expect_success 'CLI --negotiation-restrict overrides remote config' '
> +	setup_negotiation_tip server server 0 &&
> +	git -C client config --add remote.origin.negotiationRestrict alpha_1 &&
> +	git -C client config --add remote.origin.negotiationRestrict beta_1 &&
> +	ALPHA_1=$(git -C client rev-parse alpha_1) &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		origin alpha_s beta_s &&
> +	test_grep "fetch> have $ALPHA_1" trace &&
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep ! "fetch> have $BETA_1" trace
> +'
> +
>   test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
>   	git init df-conflict &&
>   	(

Again, this patch looks good to me and addresses my previous comments.

Thanks,
Matthew

On 2026-05-14 13:41, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <[email protected]>
> > In a future change, we will introduce a capability to choose specific commit
> OIDs as 'have's in fetch negotiation, with the ability to have the
> negotiator choose more 'have's to increase coverage beyond that required
> core set. The negotiator works to avoid emitting 'have's that can reach each
> other, but that logic is hidden beneath the negotiator's iterator function
> pointer ('next'). We need a way to communicate to the negotiator that we
> have picked a 'have' so it could incorporate that into its logic.
> > Add a have_sent() method to the fetch_negotiator interface. This is the
> signal that allows the negotiator to track the commit as already shown and
> can perform the proper bookkeeping to avoid emitting those objects or
> anything they can reach.
> > For our non-trivial negotiators, it is sufficient to mark these commits as
> common, so the implementation is quite simple. This logic will be exercised
> in the next change.
> > Signed-off-by: Derrick Stolee <[email protected]>

This is a new patch in v4, and is in response to the COMMON (bit 2) vs
COMMON (bit 6) issue of v3.

Having a defined have_sent() API on the negotiator, and letting the
negotiator handle its own book-keeping is much nicer and safer.

> ---
>   fetch-negotiator.h    | 9 +++++++++
>   negotiator/default.c  | 8 ++++++++
>   negotiator/noop.c     | 7 +++++++
>   negotiator/skipping.c | 8 ++++++++
>   4 files changed, 32 insertions(+)
> > diff --git a/fetch-negotiator.h b/fetch-negotiator.h
> index e348905a1f..6ca422a064 100644
> --- a/fetch-negotiator.h
> +++ b/fetch-negotiator.h
> @@ -47,6 +47,15 @@ struct fetch_negotiator {
>   	 */
>   	int (*ack)(struct fetch_negotiator *, struct commit *);
>   > +	/*
> +	 * Inform the negotiator that this commit has already been sent as
> +	 * a "have" line outside of the negotiator's control. The negotiator
> +	 * should avoid outputting it from next() and may use it to optimize
> +	 * further negotiation (e.g., by treating it and its ancestors as
> +	 * common).
> +	 */
> +	void (*have_sent)(struct fetch_negotiator *, struct commit *);
> +

Doc comment captures the contract well.

>   	void (*release)(struct fetch_negotiator *);
>   >   	/* internal use */
> diff --git a/negotiator/default.c b/negotiator/default.c
> index 116dedcf83..05ab616f39 100644
> --- a/negotiator/default.c
> +++ b/negotiator/default.c
> @@ -175,6 +175,13 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
>   	return known_to_be_common;
>   }
>   > +static void have_sent(struct fetch_negotiator *n, struct commit *c)
> +{
> +	if (repo_parse_commit(the_repository, c))
> +		return;
> +	mark_common(n->data, c, 0, 0);
> +}
> +
>   static void release(struct fetch_negotiator *n)
>   {
>   	clear_prio_queue(&((struct negotiation_state *)n->data)->rev_list);
> @@ -188,6 +195,7 @@ void default_negotiator_init(struct fetch_negotiator *negotiator)
>   	negotiator->add_tip = add_tip;
>   	negotiator->next = next;
>   	negotiator->ack = ack;
> +	negotiator->have_sent = have_sent;
>   	negotiator->release = release;
>   	negotiator->data = CALLOC_ARRAY(ns, 1);
>   	ns->rev_list.compare = compare_commits_by_commit_date;

I traced this through default.c's mark_common() to confirm this does
what we want.. sets the COMMON bit (bit 2) and propogates COMMON to
the ancestors correctly. Good!

> diff --git a/negotiator/noop.c b/negotiator/noop.c
> index 65e3c20008..edf1b456f3 100644
> --- a/negotiator/noop.c
> +++ b/negotiator/noop.c
> @@ -29,6 +29,12 @@ static int ack(struct fetch_negotiator *n UNUSED, struct commit *c UNUSED)
>   	return 0;
>   }
>   > +static void have_sent(struct fetch_negotiator *n UNUSED,
> +		      struct commit *c UNUSED)
> +{
> +	/* nothing to do */
> +}
> +
>   static void release(struct fetch_negotiator *n UNUSED)
>   {
>   	/* nothing to release */
> @@ -40,6 +46,7 @@ void noop_negotiator_init(struct fetch_negotiator *negotiator)
>   	negotiator->add_tip = add_tip;
>   	negotiator->next = next;
>   	negotiator->ack = ack;
> +	negotiator->have_sent = have_sent;
>   	negotiator->release = release;
>   	negotiator->data = NULL;
>   }

Trivial implementation of the noop negotiator. Good.

> diff --git a/negotiator/skipping.c b/negotiator/skipping.c
> index 0a272130fb..69472c58e1 100644
> --- a/negotiator/skipping.c
> +++ b/negotiator/skipping.c
> @@ -243,6 +243,13 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
>   	return known_to_be_common;
>   }
>   > +static void have_sent(struct fetch_negotiator *n, struct commit *c)
> +{
> +	if (repo_parse_commit(the_repository, c))
> +		return;
> +	mark_common(n->data, c);
> +}
> +
>   static void release(struct fetch_negotiator *n)
>   {
>   	struct data *data = n->data;
> @@ -259,6 +266,7 @@ void skipping_negotiator_init(struct fetch_negotiator *negotiator)
>   	negotiator->add_tip = add_tip;
>   	negotiator->next = next;
>   	negotiator->ack = ack;
> +	negotiator->have_sent = have_sent;
>   	negotiator->release = release;
>   	negotiator->data = CALLOC_ARRAY(data, 1);
>   	data->rev_list.compare = compare;

I traced this too, to skipping.c's mark_common(), and it sets the COMMON
bit correctly.

This patch is a welcome addition and LGTM!

Thanks,
Matthew

On 2026-05-14 13:41, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <[email protected]>
> > Add a new --negotiation-include option to 'git fetch', which ensures
> that certain ref tips are always sent as 'have' lines during fetch
> negotiation, regardless of what the negotiation algorithm selects.
> > This is useful when the repository has a large number of references, so
> the normal negotiation algorithm truncates the list. This is especially
> important in repositories with long parallel commit histories. For
> example, a repo could have a 'dev' branch for development and a
> 'release' branch for released versions. If the 'dev' branch isn't
> selected for negotiation, then it's not a big deal because there are
> many in-progress development branches with a shared history. However, if
> 'release' is not selected for negotiation, then the server may think
> that this is the first time the client has asked for that reference,
> causing a full download of its parallel commit history (and any extra
> data that may be unique to that branch). This is based on a real example
> where certain fetches would grow to 60+ GB when a release branch
> updated.
> > This option is a complement to --negotiation-restrict, which reduces the
> negotiation ref set to a specific list. In the earlier example, using
> --negotiation-restrict to focus the negotiation to 'dev' and 'release'
> would avoid those problematic downloads, but would still not allow
> advertising potentially-relevant user branches. In this way, the
> 'include' version solves the problem I mention while allowing
> negotiation to pick other references opportunistically. The two options
> can also be combined to allow the best of both worlds.

brances/branches from v3 fixed - thanks!

> The argument may be an exact ref name or a glob pattern. Non-existent
> refs are silently ignored. This behavior is also updated in the ref matching
> logic for the related --negotiation-restrict option to match.
> > The implementation outputs the requested objects as haves before the
> negotiator performs its own algorithm to choose the next haves. Use the new
> have_sent() interface to signal these have commits were sent before engaging
> with the negotiator's next() iterator.

Now references the new have_sent() API - good!

> Also add --negotiation-include to 'git pull' passthrough options.
> > Signed-off-by: Derrick Stolee <[email protected]>
> ---
>   Documentation/fetch-options.adoc | 19 +++++++
>   builtin/fetch.c                  | 32 ++++++++---
>   builtin/pull.c                   |  3 ++
>   fetch-pack.c                     | 81 +++++++++++++++++++++++++---
>   fetch-pack.h                     |  6 ++-
>   t/t5510-fetch.sh                 | 91 ++++++++++++++++++++++++++++++++
>   transport.c                      |  8 ++-
>   transport.h                      |  5 +-
>   8 files changed, 227 insertions(+), 18 deletions(-)
> > diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
> index d39cecb446..7b897a7202 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -73,6 +73,25 @@ See also the `fetch.negotiationAlgorithm` and `push.negotiate`
>   configuration variables documented in linkgit:git-config[1], and the
>   `--negotiate-only` option below.
>   > +`--negotiation-include=(<commit>|<glob>)`::
> +	Ensure that the commits at the given tips are always sent as "have"
> +	lines during fetch negotiation, regardless of what the negotiation
> +	algorithm selects.  This is useful to guarantee that common
> +	history reachable from specific refs is always considered, even
> +	when `--negotiation-restrict` restricts the set of tips or when
> +	the negotiation algorithm would otherwise skip them.
> ++
> +This option may be specified more than once; if so, each commit is sent
> +unconditionally.
> ++
> +The argument may be an exact ref name (e.g. `refs/heads/release`), an
> +object hash, or a glob pattern (e.g. `refs/heads/release/{asterisk}`).
> +The pattern syntax is the same as for `--negotiation-restrict`.
> ++
> +If `--negotiation-restrict` is used, the have set is first restricted by
> +that option and then increased to include the tips specified by
> +`--negotiation-include`.
> +

Good - the syntax placeholder matches --negotiate-restrict, and we
mention object hashes being valid input.

>   `--negotiate-only`::
>   	Do not fetch anything from the server, and instead print the
>   	ancestors of the provided `--negotiation-restrict=` arguments,
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index a957739f37..6b456b3689 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -99,6 +99,7 @@ static struct transport *gsecondary;
>   static struct refspec refmap = REFSPEC_INIT_FETCH;
>   static struct string_list server_options = STRING_LIST_INIT_DUP;
>   static struct string_list negotiation_restrict = STRING_LIST_INIT_NODUP;
> +static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
>   >   struct fetch_config {
>   	enum display_format display_format;
> @@ -1534,23 +1535,28 @@ static int add_oid(const struct reference *ref, void *cb_data)
>   	return 0;
>   }
>   > -static void add_negotiation_restrict_tips(struct git_transport_options *smart_options)
> +static void add_negotiation_tips(struct string_list *input_list,
> +				 struct oid_array **output_list)
>   {
>   	struct oid_array *oids = xcalloc(1, sizeof(*oids));
>   	int i;
>   > -	for (i = 0; i < negotiation_restrict.nr; i++) {
> -		const char *s = negotiation_restrict.items[i].string;
> +	for (i = 0; i < input_list->nr; i++) {
> +		const char *s = input_list->items[i].string;
>   		struct refs_for_each_ref_options opts = {
>   			.pattern = s,
>   		};
>   		int old_nr;
>   		if (!has_glob_specials(s)) {
>   			struct object_id oid;
> +
> +			/* Ignore missing reference. */
>   			if (repo_get_oid(the_repository, s, &oid))
> -				die(_("%s is not a valid object"), s);
> +				continue;
> +			/* Fail on missing object pointed by ref. */
>   			if (!odb_has_object(the_repository->objects, &oid, 0))
>   				die(_("the object %s does not exist"), s);
> +
>   			oid_array_append(oids, &oid);
>   			continue;
>   		}
> @@ -1561,7 +1567,7 @@ static void add_negotiation_restrict_tips(struct git_transport_options *smart_op
>   			warning(_("ignoring %s=%s because it does not match any refs"),
>   				"--negotiation-restrict", s);
>   	}
> -	smart_options->negotiation_restrict_tips = oids;
> +	*output_list = oids;
>   }
>   >   static struct transport *prepare_transport(struct remote *remote, int deepen,

Great! Now we have a unified implementation that takes a string_list
and ouputs an oid_array, used for both restrict and include lists.
We pre-resolve so that fetch-pack gets the oid_array so the ref
resolution happens in the same layer for both.

Just one small issue I see - the function emits a warning for
"--negotiate-restrict" when we don't match a ref, even if this is
handling a --negotiate-include value. Perhaps we should pass the
option name in as a parameter?

  add_negotiation_tips(&negotiation_include,
                       "--negotiation-include",
                       &transport->smart_opt->negotiation_include_tips);

> @@ -1597,7 +1603,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
>   	}
>   	if (negotiation_restrict.nr) {
>   		if (transport->smart_options)
> -			add_negotiation_restrict_tips(transport->smart_options);
> +			add_negotiation_tips(&negotiation_restrict,
> +					     &transport->smart_options->negotiation_restrict_tips);
>   		else
>   			warning(_("ignoring %s because the protocol does not support it"),
>   				"--negotiation-restrict");
> @@ -1606,7 +1613,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
>   		for_each_string_list_item(item, &remote->negotiation_restrict)
>   			string_list_append(&negotiation_restrict, item->string);
>   		if (transport->smart_options)
> -			add_negotiation_restrict_tips(transport->smart_options);
> +			add_negotiation_tips(&negotiation_restrict,
> +					     &transport->smart_options->negotiation_restrict_tips);
>   		else {
>   			struct strbuf config_name = STRBUF_INIT;
>   			strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name);
> @@ -1615,6 +1623,14 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
>   			strbuf_release(&config_name);
>   		}
>   	}
> +	if (negotiation_include.nr) {
> +		if (transport->smart_options)
> +			add_negotiation_tips(&negotiation_include,
> +					     &transport->smart_options->negotiation_include_tips);
> +		else
> +			warning(_("ignoring %s because the protocol does not support it"),
> +				"--negotiation-include");
> +	}
>   	return transport;
>   }

Having the shared helper makes this much nicer. Good!

> @@ -2582,6 +2598,8 @@ int cmd_fetch(int argc,
>   		OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_restrict, N_("revision"),
>   				N_("report that we have only objects reachable from this object")),
>   		OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
> +		OPT_STRING_LIST(0, "negotiation-include", &negotiation_include, N_("revision"),
> +				N_("ensure this ref is always sent as a negotiation have")),
>   		OPT_BOOL(0, "negotiate-only", &negotiate_only,
>   			 N_("do not fetch a packfile; instead, print ancestors of negotiation tips")),
>   		OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
> diff --git a/builtin/pull.c b/builtin/pull.c
> index cc6ce485fc..d49b09114a 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -1000,6 +1000,9 @@ int cmd_pull(int argc,
>   			N_("report that we have only objects reachable from this object"),
>   			0),
>   		OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
> +		OPT_PASSTHRU_ARGV(0, "negotiation-include", &opt_fetch, N_("revision"),
> +			N_("ensure this ref is always sent as a negotiation have"),
> +			0),
>   		OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
>   			 N_("check for forced-updates on all updated branches")),
>   		OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,

LGTM

> diff --git a/fetch-pack.c b/fetch-pack.c
> index baf239adf9..96071434b8 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -25,6 +25,7 @@
>   #include "oidset.h"
>   #include "packfile.h"
>   #include "odb.h"
> +#include "object-name.h"
>   #include "path.h"
>   #include "connected.h"
>   #include "fetch-negotiator.h"
> @@ -332,6 +333,21 @@ static void send_filter(struct fetch_pack_args *args,
>   	}
>   }
>   > +static void add_oids_to_set(const struct oid_array *array,
> +			    struct oidset *set)
> +{
> +	if (!array)
> +		return;
> +
> +	for (size_t i = 0; i < array->nr; i++) {
> +		struct object_id *oid = &array->oid[i];
> +		if (!odb_has_object(the_repository->objects, oid, 0))
> +			die(_("the object %s does not exist"), oid_to_hex(oid));
> +
> +		oidset_insert(set, oid);
> +	}
> +}
> +

Nice - this is much simpler since ref resolution has been hoisted up
and we only deal with pre-resolved OIDs.

>   static int find_common(struct fetch_negotiator *negotiator,
>   		       struct fetch_pack_args *args,
>   		       int fd[2], struct object_id *result_oid,
> @@ -347,6 +363,7 @@ static int find_common(struct fetch_negotiator *negotiator,
>   	struct strbuf req_buf = STRBUF_INIT;
>   	size_t state_len = 0;
>   	struct packet_reader reader;
> +	struct oidset negotiation_include_oids = OIDSET_INIT;
>   >   	if (args->stateless_rpc && multi_ack == 1)
>   		die(_("the option '%s' requires '%s'"), "--stateless-rpc", "multi_ack_detailed");
> @@ -474,6 +491,27 @@ static int find_common(struct fetch_negotiator *negotiator,
>   	trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository);
>   	flushes = 0;
>   	retval = -1;
> +
> +	/* Send unconditional haves from --negotiation-include */
> +	add_oids_to_set(args->negotiation_include_tips,
> +			&negotiation_include_oids);
> +	if (oidset_size(&negotiation_include_oids)) {
> +		struct oidset_iter iter;
> +		oidset_iter_init(&negotiation_include_oids, &iter);
> +
> +		while ((oid = oidset_iter_next(&iter))) {
> +			struct commit *commit;
> +			packet_buf_write(&req_buf, "have %s\n",
> +					 oid_to_hex(oid));
> +			print_verbose(args, "have %s", oid_to_hex(oid));
> +			count++;
> +
> +			commit = lookup_commit(the_repository, oid);
> +			if (commit)
> +				negotiator->have_sent(negotiator, commit);
> +		}
> +	}
> +
>   	while ((oid = negotiator->next(negotiator))) {
>   		packet_buf_write(&req_buf, "have %s\n", oid_to_hex(oid));
>   		print_verbose(args, "have %s", oid_to_hex(oid));
> @@ -584,6 +622,7 @@ done:
>   		flushes++;
>   	}
>   	strbuf_release(&req_buf);
> +	oidset_clear(&negotiation_include_oids);
>   >   	if (!got_ready || !no_done)
>   		consume_shallow_list(args, &reader);
> @@ -1305,11 +1344,27 @@ static void add_common(struct strbuf *req_buf, struct oidset *common)
>   >   static int add_haves(struct fetch_negotiator *negotiator,
>   		     struct strbuf *req_buf,
> -		     int *haves_to_send)
> +		     int *haves_to_send,
> +		     struct oidset *negotiation_include_oids)
>   {
>   	int haves_added = 0;
>   	const struct object_id *oid;
>   > +	/* Send unconditional haves from --negotiation-include */
> +	if (negotiation_include_oids) {
> +		struct oidset_iter iter;
> +		oidset_iter_init(negotiation_include_oids, &iter);
> +
> +		while ((oid = oidset_iter_next(&iter))) {
> +			struct commit *commit = lookup_commit(the_repository, oid);
> +			if (commit) {
> +				packet_buf_write(req_buf, "have %s\n",
> +						 oid_to_hex(oid));
> +				negotiator->have_sent(negotiator, commit);
> +			}
> +		}
> +	}
> +
>   	while ((oid = negotiator->next(negotiator))) {
>   		packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid));
>   		if (++haves_added >= *haves_to_send)

Here we're using the new have_sent() API and replaces the manual COMMON
bit flipping.

> @@ -1358,7 +1413,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
>   			      struct fetch_pack_args *args,
>   			      const struct ref *wants, struct oidset *common,
>   			      int *haves_to_send, int *in_vain,
> -			      int sideband_all, int seen_ack)
> +			      int sideband_all, int seen_ack,
> +			      struct oidset *negotiation_include_oids)
>   {
>   	int haves_added;
>   	int done_sent = 0;
> @@ -1413,7 +1469,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
>   	/* Add all of the common commits we've found in previous rounds */
>   	add_common(&req_buf, common);
>   > -	haves_added = add_haves(negotiator, &req_buf, haves_to_send);
> +	haves_added = add_haves(negotiator, &req_buf, haves_to_send,
> +			       negotiation_include_oids);
>   	*in_vain += haves_added;
>   	trace2_data_intmax("negotiation_v2", the_repository, "haves_added", haves_added);
>   	trace2_data_intmax("negotiation_v2", the_repository, "in_vain", *in_vain);
> @@ -1657,6 +1714,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
>   	struct ref *ref = copy_ref_list(orig_ref);
>   	enum fetch_state state = FETCH_CHECK_LOCAL;
>   	struct oidset common = OIDSET_INIT;
> +	struct oidset negotiation_include_oids = OIDSET_INIT;
>   	struct packet_reader reader;
>   	int in_vain = 0, negotiation_started = 0;
>   	int negotiation_round = 0;
> @@ -1729,6 +1787,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
>   				state = FETCH_SEND_REQUEST;
>   >   			mark_tips(negotiator, args->negotiation_restrict_tips);
> +			add_oids_to_set(args->negotiation_include_tips,
> +					&negotiation_include_oids);
>   			for_each_cached_alternate(negotiator,
>   						  insert_one_alternate_object);
>   			break;
> @@ -1747,7 +1807,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
>   					       &common,
>   					       &haves_to_send, &in_vain,
>   					       reader.use_sideband,
> -					       seen_ack)) {
> +					       seen_ack,
> +					       &negotiation_include_oids)) {
>   				trace2_region_leave_printf("negotiation_v2", "round",
>   							   the_repository, "%d",
>   							   negotiation_round);
> @@ -1883,6 +1944,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
>   		negotiator->release(negotiator);
>   >   	oidset_clear(&common);
> +	oidset_clear(&negotiation_include_oids);
>   	return ref;
>   }

The v2 wiring looks correct.

> @@ -2181,12 +2243,14 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
>   			   const struct string_list *server_options,
>   			   int stateless_rpc,
>   			   int fd[],
> -			   struct oidset *acked_commits)
> +			   struct oidset *acked_commits,
> +			   const struct oid_array *negotiation_include_tips)
>   {
>   	struct fetch_negotiator negotiator;
>   	struct packet_reader reader;
>   	struct object_array nt_object_array = OBJECT_ARRAY_INIT;
>   	struct strbuf req_buf = STRBUF_INIT;
> +	struct oidset negotiation_include_oids = OIDSET_INIT;
>   	int haves_to_send = INITIAL_FLUSH;
>   	int in_vain = 0;
>   	int seen_ack = 0;
> @@ -2197,6 +2261,9 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
>   	fetch_negotiator_init(the_repository, &negotiator);
>   	mark_tips(&negotiator, negotiation_restrict_tips);
>   > +	add_oids_to_set(negotiation_include_tips,
> +			&negotiation_include_oids);
> +
>   	packet_reader_init(&reader, fd[0], NULL, 0,
>   			   PACKET_READ_CHOMP_NEWLINE |
>   			   PACKET_READ_DIE_ON_ERR_PACKET);
> @@ -2221,7 +2288,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
>   >   		packet_buf_write(&req_buf, "wait-for-done");
>   > -		haves_added = add_haves(&negotiator, &req_buf, &haves_to_send);
> +		haves_added = add_haves(&negotiator, &req_buf, &haves_to_send,
> +				       &negotiation_include_oids);
>   		in_vain += haves_added;
>   		if (!haves_added || (seen_ack && in_vain >= MAX_IN_VAIN))
>   			last_iteration = 1;
> @@ -2273,6 +2341,7 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
>   >   	clear_common_flag(acked_commits);
>   	object_array_clear(&nt_object_array);
> +	oidset_clear(&negotiation_include_oids);
>   	negotiator.release(&negotiator);
>   	strbuf_release(&req_buf);
>   }
> diff --git a/fetch-pack.h b/fetch-pack.h
> index 6c70c942c2..6d0dec7f41 100644
> --- a/fetch-pack.h
> +++ b/fetch-pack.h
> @@ -19,9 +19,10 @@ struct fetch_pack_args {
>   >   	/*
>   	 * If not NULL, during packfile negotiation, fetch-pack will send "have"
> -	 * lines only with these tips and their ancestors.
> +	 * lines for all _include_ tips and then a subset of the _restrict_ tips.
>   	 */
>   	const struct oid_array *negotiation_restrict_tips;
> +	const struct oid_array *negotiation_include_tips;
>   >   	unsigned deepen_relative:1;
>   	unsigned quiet:1;
> @@ -93,7 +94,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
>   			   const struct string_list *server_options,
>   			   int stateless_rpc,
>   			   int fd[],
> -			   struct oidset *acked_commits);
> +			   struct oidset *acked_commits,
> +			   const struct oid_array *negotiation_include_tips);
>   >   /*
>    * Print an appropriate error message for each sought ref that wasn't
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index eff3ce8e2d..bc2e2af959 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -1460,6 +1460,16 @@ EOF
>   	test_cmp fatal-expect fatal-actual
>   '
>   > +test_expect_success '--negotiation-tip ignores missing refs and invalid hashes' '
> +	setup_negotiation_tip server server 0 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-tip=alpha_1 --negotiation-tip=beta_1 \
> +		--negotiation-tip=no-such-ref \
> +		--negotiation-tip=invalid-hash \
> +		origin alpha_s beta_s &&
> +	check_negotiation_tip
> +'
> +

Good! This checks for missing refs (not just invalid ones with all
zeros).

>   test_expect_success '--negotiation-restrict limits "have" lines sent' '
>   	setup_negotiation_tip server server 0 &&
>   	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> @@ -1511,6 +1521,87 @@ test_expect_success 'CLI --negotiation-restrict overrides remote config' '
>   	test_grep ! "fetch> have $BETA_1" trace
>   '
>   > +test_expect_success '--negotiation-include includes configured refs as haves' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		--negotiation-include=refs/tags/beta_1 \
> +		origin alpha_s beta_s &&
> +
> +	ALPHA_1=$(git -C client rev-parse alpha_1) &&
> +	test_grep "fetch> have $ALPHA_1" trace &&
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace
> +'
> +
> +test_expect_success '--negotiation-include works with glob patterns' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		--negotiation-include="refs/tags/beta_*" \
> +		origin alpha_s beta_s &&
> +
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace &&
> +	BETA_2=$(git -C client rev-parse beta_2) &&
> +	test_grep "fetch> have $BETA_2" trace
> +'
> +
> +test_expect_success '--negotiation-include is additive with negotiation' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-include=refs/tags/beta_1 \
> +		origin alpha_s beta_s &&
> +
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace
> +'
> +
> +test_expect_success '--negotiation-include ignores non-existent refs silently' '
> +	setup_negotiation_tip server server 0 &&
> +
> +	git -C client fetch --quiet \
> +		--negotiation-restrict=alpha_1 \
> +		--negotiation-include=refs/tags/nonexistent \
> +		origin alpha_s beta_s 2>err &&
> +	test_must_be_empty err
> +'
> +
> +test_expect_success '--negotiation-include avoids duplicates with negotiator' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	ALPHA_1=$(git -C client rev-parse alpha_1) &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		--negotiation-include=refs/tags/alpha_1 \
> +		origin alpha_s beta_s &&
> +
> +	test_grep "fetch> have $ALPHA_1" trace >matches &&
> +	test_line_count = 1 matches
> +'
> +
> +test_expect_success '--negotiation-include avoids duplicates with v0' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	ALPHA_1=$(git -C client rev-parse alpha_1) &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client \
> +		-c protocol.version=0 fetch \
> +		--negotiation-restrict=alpha_1 \
> +		--negotiation-include=refs/tags/alpha_1 \
> +		origin alpha_s beta_s &&
> +
> +	test_grep "fetch> have $ALPHA_1" trace >matches &&
> +	test_line_count = 1 matches
> +'
> +
>   test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
>   	git init df-conflict &&
>   	(
> diff --git a/transport.c b/transport.c
> index a3051f6733..fa54928966 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -464,6 +464,7 @@ static int fetch_refs_via_pack(struct transport *transport,
>   	args.stateless_rpc = transport->stateless_rpc;
>   	args.server_options = transport->server_options;
>   	args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
> +	args.negotiation_include_tips = data->options.negotiation_include_tips;
>   	args.reject_shallow_remote = transport->smart_options->reject_shallow;
>   >   	if (!data->finished_handshake) {
> @@ -495,7 +496,8 @@ static int fetch_refs_via_pack(struct transport *transport,
>   					      transport->server_options,
>   					      transport->stateless_rpc,
>   					      data->fd,
> -					      data->options.acked_commits);
> +					      data->options.acked_commits,
> +					      data->options.negotiation_include_tips);
>   			ret = 0;
>   		}
>   		goto cleanup;
> @@ -983,6 +985,10 @@ static int disconnect_git(struct transport *transport)
>   		oid_array_clear(data->options.negotiation_restrict_tips);
>   		free(data->options.negotiation_restrict_tips);
>   	}
> +	if (data->options.negotiation_include_tips) {
> +		oid_array_clear(data->options.negotiation_include_tips);
> +		free(data->options.negotiation_include_tips);
> +	}
>   	list_objects_filter_release(&data->options.filter_options);
>   	oid_array_clear(&data->extra_have);
>   	oid_array_clear(&data->shallow);
> diff --git a/transport.h b/transport.h
> index cdeb33c16f..97d905ecc0 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -40,13 +40,14 @@ struct git_transport_options {
>   >   	/*
>   	 * This is only used during fetch. See the documentation of
> -	 * negotiation_restrict_tips in struct fetch_pack_args.
> +	 * these member names in struct fetch_pack_args.
>   	 *
> -	 * This field is only supported by transports that support connect or
> +	 * These fields are only supported by transports that support connect or
>   	 * stateless_connect. Set this field directly instead of using
>   	 * transport_set_option().
>   	 */
>   	struct oid_array *negotiation_restrict_tips;
> +	struct oid_array *negotiation_include_tips;
>   >   	/*
>   	 * If allocated, whenever transport_fetch_refs() is called, add known

Overall this version of the patch is much better IMO! Just that one
comment about the incorrect option name in the warning message.

Thanks,
Matthew

On 2026-05-14 13:41, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <[email protected]>
> > Add a new 'remote.<name>.negotiationInclude' multi-valued config option that
> provides default values for --negotiation-include when no
> --negotiation-include arguments are specified over the command line.  This
> is a mirror of how 'remote.<name>.negotiationRestrict' specifies defaults
> for the --negotiation-restrict arguments.
> > Each value is either an exact ref name or a glob pattern whose tips should
> always be sent as 'have' lines during negotiation. The config values are
> resolved through the same resolve_negotiation_include() codepath as the CLI
> options.
> > This option is additive with the normal negotiation process: the negotiation
> algorithm still runs and advertises its own selected commits, but the refs
> matching the config are sent unconditionally on top of those heuristically
> selected commits.
> > Similar to the negotiationRestrict config, an empty value resets the value
> list to allow ignoring earlier config values, such as those that might be
> set in system or global config.
> > Signed-off-by: Derrick Stolee <[email protected]>
> ---
>   Documentation/config/remote.adoc | 27 ++++++++++++++++++
>   Documentation/fetch-options.adoc |  4 +++
>   builtin/fetch.c                  | 11 +++++++
>   remote.c                         |  5 ++++
>   remote.h                         |  1 +
>   t/t5510-fetch.sh                 | 49 ++++++++++++++++++++++++++++++++
>   6 files changed, 97 insertions(+)
> > diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
> index 4dcf81fbce..9ae20e4379 100644
> --- a/Documentation/config/remote.adoc
> +++ b/Documentation/config/remote.adoc
> @@ -125,6 +125,33 @@ values are not used.
>   Blank values signal to ignore all previous values, allowing a reset of
>   the list from broader config scenarios.
>   > +remote.<name>.negotiationInclude::
> +	When negotiating with this remote during `git fetch`, the client
> +	advertises a list of commits that exist locally.  In repos with
> +	many references, this list of "haves" can be truncated. Depending
> +	on data shape, dropping certain references may be expensive. This
> +	multi-valued config option specifies references, commit hashes,
> +	or ref pattern globs whose tips should always be sent as "have"
> +	commits during fetch negotiation with this remote.
> ++
> +Each value is either an exact ref name (e.g. `refs/heads/release`), a
> +commit hash, or a glob pattern (e.g. `refs/heads/release/*`).  The
> +pattern syntax is the same as for `--negotiation-include`.

Thanks - references the correct cross-referenced option I raised in v3.
Commit hashes are also explicitly mentioned - good.

> +These config values are used as defaults for the `--negotiation-include`
> +command-line option.  If `--negotiation-include` is specified on the
> +command line, then the config values are not used.
> ++
> +This option is additive with the normal negotiation process: the
> +negotiation algorithm still runs and advertises its own selected commits,
> +but the refs matching `remote.<name>.negotiationInclude` are sent
> +unconditionally on top of those heuristically selected commits.  This
> +option is also used during push negotiation when `push.negotiate` is
> +enabled.

One thing to mention: the "also used during push negotiation" sentence
is added here, and then I see in the next patch (patch 8) we're adding
another sentence "these values also influence negotiation during git
push" to this same block.

Perhaps consider dropping the additional 'push' sentence here (patch 7)
and let patch 8 add just its own sentence about push? Not too pressing.

> ++
> +Blank values signal to ignore all previous values, allowing a reset of
> +the list from broader config scenarios.
> +
>   remote.<name>.followRemoteHEAD::
>   	How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
>   	when fetching using the configured refspecs of a remote.
> diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
> index 7b897a7202..8074004377 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -91,6 +91,10 @@ The pattern syntax is the same as for `--negotiation-restrict`.
>   If `--negotiation-restrict` is used, the have set is first restricted by
>   that option and then increased to include the tips specified by
>   `--negotiation-include`.
> ++
> +If this option is not specified on the command line, then any
> +`remote.<name>.negotiationInclude` config values for the current remote
> +are used instead.
>   >   `--negotiate-only`::
>   	Do not fetch anything from the server, and instead print the
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 6b456b3689..2308cab377 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1630,6 +1630,17 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
>   		else
>   			warning(_("ignoring %s because the protocol does not support it"),
>   				"--negotiation-include");
> +	} else if (remote->negotiation_include.nr) {
> +		if (transport->smart_options) {
> +			add_negotiation_tips(&remote->negotiation_include,
> +					     &transport->smart_options->negotiation_include_tips);
> +		} else {
> +			struct strbuf config_name = STRBUF_INIT;
> +			strbuf_addf(&config_name, "remote.%s.negotiationInclude", remote->name);
> +			warning(_("ignoring %s because the protocol does not support it"),
> +				config_name.buf);
> +			strbuf_release(&config_name);
> +		}
>   	}
>   	return transport;
>   }
> diff --git a/remote.c b/remote.c
> index 620086e16e..6fb5758820 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -153,6 +153,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
>   	refspec_init_fetch(&ret->fetch);
>   	string_list_init_dup(&ret->server_options);
>   	string_list_init_dup(&ret->negotiation_restrict);
> +	string_list_init_dup(&ret->negotiation_include);
>   >   	ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
>   		   remote_state->remotes_alloc);
> @@ -181,6 +182,7 @@ static void remote_clear(struct remote *remote)
>   	FREE_AND_NULL(remote->http_proxy_authmethod);
>   	string_list_clear(&remote->server_options, 0);
>   	string_list_clear(&remote->negotiation_restrict, 0);
> +	string_list_clear(&remote->negotiation_include, 0);
>   }
>   >   static void add_merge(struct branch *branch, const char *name)
> @@ -567,6 +569,9 @@ static int handle_config(const char *key, const char *value,
>   	} else if (!strcmp(subkey, "negotiationrestrict")) {
>   		return parse_transport_option(key, value,
>   					      &remote->negotiation_restrict);
> +	} else if (!strcmp(subkey, "negotiationinclude")) {
> +		return parse_transport_option(key, value,
> +					      &remote->negotiation_include);
>   	} else if (!strcmp(subkey, "followremotehead")) {
>   		const char *no_warn_branch;
>   		if (!strcmp(value, "never"))

Good - uses the parse_transport_options() like with the earlier patch.

> diff --git a/remote.h b/remote.h
> index e6ec37c393..d8809b6991 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -118,6 +118,7 @@ struct remote {
>   >   	struct string_list server_options;
>   	struct string_list negotiation_restrict;
> +	struct string_list negotiation_include;
>   >   	enum follow_remote_head_settings follow_remote_head;
>   	const char *no_warn_branch;
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index bc2e2af959..33f61ac12a 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -1587,6 +1587,55 @@ test_expect_success '--negotiation-include avoids duplicates with negotiator' '
>   	test_line_count = 1 matches
>   '
>   > +test_expect_success 'remote.<name>.negotiationInclude used as default for --negotiation-include' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	# test the reset of the list on an empty value
> +	git -C client config --add remote.origin.negotiationInclude refs/tags/alpha_1 &&
> +	git -C client config --add remote.origin.negotiationInclude "" &&
> +	git -C client config --add remote.origin.negotiationInclude refs/tags/beta_1 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=beta_2 \
> +		origin alpha_s beta_s &&
> +
> +	ALPHA_1=$(git -C client rev-parse alpha_1) &&
> +	test_grep ! "fetch> have $ALPHA_1" trace &&
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace
> +'

Great! Now this test will catch failures to the reset-list behaviour
by using a different CLI option ref name than is in the config list.

> +
> +test_expect_success 'remote.<name>.negotiationInclude works with glob patterns' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	git -C client config --add remote.origin.negotiationInclude "refs/tags/beta_*" &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		origin alpha_s beta_s &&
> +
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace &&
> +	BETA_2=$(git -C client rev-parse beta_2) &&
> +	test_grep "fetch> have $BETA_2" trace
> +'
> +
> +test_expect_success 'CLI --negotiation-include overrides remote.<name>.negotiationInclude' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	git -C client config --add remote.origin.negotiationInclude refs/tags/beta_2 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		--negotiation-include=refs/tags/beta_1 \
> +		origin alpha_s beta_s &&
> +
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace &&
> +	BETA_2=$(git -C client rev-parse beta_2) &&
> +	test_grep ! "fetch> have $BETA_2" trace
> +'
> +
>   test_expect_success '--negotiation-include avoids duplicates with v0' '
>   	test_when_finished rm -f trace &&
>   	setup_negotiation_tip server server 0 &&

Overall this patch addresses my comments from v3's review. Just the
minor issue with forward reference to 'git push' behaviour in the docs.

Thanks,
Matthew